Add workspaces - #68
Conversation
Mira PR WalkthroughThis PR adds the ability to specify a target lane override when landing a workspace, via a new graph LR
cli["src/cli.ts"]
land["src/workspace-land.ts"]
state["src/workspace-state.ts"]
test["tests/workspace.test.ts"]
cli -- "--lane flag" --> land
land -- "writes landedLane" --> state
test -- "exercises" --> land
test -- "verifies" --> state
Confidence: 4/5 ◉◉◉◉○ Safe with minor fixes
3 files reviewed
|
There was a problem hiding this comment.
Mira Review Summary
This is a large, well-structured PR implementing composed workspaces for Patchlane 0.5.3. The architecture is sound: separating composition from sync, pinning lane SHAs at workspace creation time, and enforcing exact tree equality via round-trip validation. Two issues need attention before merge: (1) the CLI workspace land handler does not propagate --origin-remote-name, --upstream-remote-name, or UPSTREAM_REMOTE_URL options to landWorkspace(), making those CLI flags silently ignored; (2) no tests were added alongside the ~1,700 lines of new source code — the workspace creation, landing (including projection, recomposition, and exact tree comparison), state persistence, and cleanup paths all need coverage.
Key Issues
| Issue | Location | |
|---|---|---|
| 🔴 | CLI options --origin-remote-name, --upstream-remote-name, and UPSTREAM_REMOTE_URL are not propagated from the CLI workspace command handler to landWorkspace(), making the advertised CLI flags silently ineffective for the land action. |
src/cli.ts:170 |
| 🔴 | No tests added for ~1,700 lines of new workspace logic including commit replay, exact tree comparison, atomic state persistence, and cleanup error paths. | src/workspace-create.ts:118 |
There was a problem hiding this comment.
Mira Review Summary
One issue found: the local lane branch is updated to candidateLaneSha before the push attempt when --push is used. If the push fails, the local branch has already been mutated while the workspace state remains unlanded, creating an inconsistent state. The local ref update should move after a successful push, or at minimum be rolled back on push failure.
Key Issues
| Issue | Location | |
|---|---|---|
| 🔴 | Local lane branch refs/heads/${targetLane} is updated unconditionally at line 403 before the push block. If the push fails, the local branch has already been mutated to candidateLaneSha with no corresponding workspace state update, leaving the system in an inconsistent state. |
src/workspace-land.ts:403 |
No description provided.